Skip to content

(compat) Fix incorrect usages of storage service at Runtime and Loader layer#25722

Merged
agarwal-navin merged 1 commit intomicrosoft:mainfrom
agarwal-navin:cleanupStorageService
Oct 21, 2025
Merged

(compat) Fix incorrect usages of storage service at Runtime and Loader layer#25722
agarwal-navin merged 1 commit intomicrosoft:mainfrom
agarwal-navin:cleanupStorageService

Conversation

@agarwal-navin
Copy link
Contributor

@agarwal-navin agarwal-navin commented Oct 20, 2025

There are a couple of incorrect usages of the storage service at the Runtime and Loader layer after IContainerStorageService and IRuntimeStorageService were added by #25057. This PR fixes them.

Also, removed a test that was ensuring getSnapshotTree cannot be accessed from data store context. With #25057, getSnapshotTree is not accesible from data store context's storage anymore so this test is not needed.

Copilot AI review requested due to automatic review settings October 20, 2025 23:39
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes incorrect usages of storage service types at the Runtime and Loader layers following the introduction of IContainerStorageService and IRuntimeStorageService. The changes ensure the correct storage service interfaces are used in mock implementations and type annotations.

Key Changes

  • Updated mock storage service implementations to use IContainerStorageService instead of IRuntimeStorageService
  • Corrected import statements to reference the appropriate storage service types
  • Removed a test that verified getSnapshotTree inaccessibility from data store context, which is now obsolete

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
packages/test/test-end-to-end-tests/src/test/deRehydrateContainerTests.spec.ts Removed obsolete test case that checked storage access restrictions in detached containers
packages/runtime/container-runtime/src/test/containerRuntime.spec.ts Updated mock storage service types from IRuntimeStorageService to IContainerStorageService
packages/loader/container-loader/src/container.ts Corrected parameter type in initializeProtocolStateFromSnapshot from IDocumentStorageService to IContainerStorageService

@github-actions github-actions bot added area: loader Loader related issues area: runtime Runtime related issues area: tests Tests to add, test infrastructure improvements, etc base: main PRs targeted against main branch labels Oct 20, 2025
Copy link
Contributor

@jason-ha jason-ha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe strike the last sentence of the PR description.

Also, even if we were to keep this, we would have to cast data store context's storage any or one of the storage service interfaces from upper layer which again doesn't test anything useful.

I think it just adds confusion. It is enough to say it isn't useful.

@agarwal-navin agarwal-navin merged commit 6a3e9a2 into microsoft:main Oct 21, 2025
37 checks passed
@agarwal-navin agarwal-navin deleted the cleanupStorageService branch October 21, 2025 00:37
anthony-murphy-agent pushed a commit to anthony-murphy-agent/FluidFramework that referenced this pull request Jan 14, 2026
…r layer (microsoft#25722)

There are a couple of incorrect usages of the storage service at the
Runtime and Loader layer after `IContainerStorageService` and
`IRuntimeStorageService` were added by
microsoft#25057. This PR fixes
them.

Also, removed a test that was ensuring `getSnapshotTree` cannot be
accessed from data store context. With
microsoft#25057,
`getSnapshotTree` is not accesible from data store context's storage
anymore so this test is not needed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: loader Loader related issues area: runtime Runtime related issues area: tests Tests to add, test infrastructure improvements, etc base: main PRs targeted against main branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants